refactor: Supports cleaning resources for static projects with `test-acc-tf-p-keep` prefix by EspenAlbert · Pull Request #3269 · mongodb/terraform-provider-mongodbatlas

EspenAlbert · 2025-04-10T19:39:06Z

Description

Related issue: https://jira.mongodb.org/browse/CLOUDP-314965

Supports cleaning projects with "test-acc-tf-p-keep" prefix, that shouldn't be deleted but have resources removed when running the cleanup (the project should be empty after tests).

We have various tests that depend on using specific projects with feature flags enabled.
However, these projects risk having extra resources that are not cleaned.

The only resources found for deletion:

test-acc-tf-p-keep-ear-AWS-private-endpoint (6790e57a9b41416f5c216fee) # 3 stream instances
test-acc-tf-p-keep-ear-private-endpoint (66d83bcc1fe1835125c52422) # no resources
test-acc-tf-p-keep-encryption-at-rest (67810c93a7dcf40b5f0db4c2) # no resources

Sweeper Alternative considered

tl;dr: Uses the same mechanism as go test but with a -sweep={regionName} flag. I consider our current cleanup test to be easier to adapt than adding custom sweepers and a new CI workflow.

Docs: https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/sweepers
AWS Example invoke and code
Advantage over CheckDestroy: If a test panics or the process terminates unexpectedly, checkDestroy never runs.
CI Integration: Sweepers are designed to be run in CI pipelines on schedules, independent of test runs.
The sweeper function must accept a region string parameter and return an error

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

… but have resources removed

lantoli · 2025-04-11T07:22:27Z

internal/testutil/clean/org_clean_test.go

 		"test-acc-tf-p-keep",
 	}
+	keepProjectEmptyPrefixes = []string{
+		"test-acc-tf-p-empty",


is there any case where we want to keep the project without deleting anything in it? are they with prefix test-acc-tf-p-keep? also I can imagine that in keep ones we want to keep some type of resource (e.g. private endpoint) but delete others (like clusters). so not sure if it will be more general to use only one prefix project, e.g. test-acc-tf-p-keep, and then use "keep" in resources we want to keep inside, e.g. cluster "test-acc-tf-c-keep-mycluster would me not to delete that cluster.

Agree we can preserve test-acc-tf-p-keep prefix for all cases.
Not sure how complex it would be to start adding different keep prefixes for each resource type, as for this PR maybe we can stick to test-acc-tf-p-keep (preserves any resources) and test-acc-tf-p-keep-empty (removes all resources)?

@AgustinBettati FYI, see also comments in the doc

Changed test-acc-tf-p-keep instead

internal/testutil/clean/org_clean_test.go

AgustinBettati · 2025-04-11T08:51:47Z

internal/testutil/clean/org_clean_test.go

 	keptPrefixes = []string{
 		"test-acc-tf-p-keep",
 	}
+	keepProjectEmptyPrefixes = []string{


changes LGTM, would only double check that for the following projects we don't have any static resources we are relying on that could be deleted. (I dont believe its the case)
EAR-example (6792ac6ceaa5601d0f45dc95)
test-acc-tf-p-keep-ear-AWS-private-endpoint (6790e57a9b41416f5c216fee)
test-acc-tf-p-keep-ear-private-endpoint (66d83bcc1fe1835125c52422)
test-acc-tf-p-keep-encryption-at-rest (67810c93a7dcf40b5f0db4c2)

Checked. Only test-acc-tf-p-keep-ear-AWS-private-endpoint has a few steam_instances, but those are ok to delete

github-actions · 2025-04-17T00:28:15Z

This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy!

…nly found a few stream-instances for now)

internal/testutil/clean/org_clean_test.go

lantoli · 2025-04-22T16:17:54Z

internal/testutil/clean/org_clean_test.go

@@ -204,11 +211,6 @@ func removeProjectResources(ctx context.Context, t *testing.T, dryRun bool, clie
 }

 func projectSkipReason(p *admin.Group, skipProjectsAfter time.Time, onlyEmpty bool) string {


what's the difference between projectSkipReason and projectNoDeleteReason?

projectSkipReason avoids cleaning the project alltogether. (All non bot projects are not cleaned by default)
skipProjectDelete cleans resources but doesn't delete the project.

thanks for the clarification

Co-authored-by: Leo Antoli <430982+lantoli@users.noreply.github.com>

AgustinBettati

LGTM, just a suggestion for improving clarity

AgustinBettati · 2025-04-23T09:05:01Z

internal/testutil/clean/org_clean_test.go

 	t.Logf("will try to delete %d projects:", len(projectsToDelete))
 	slices.Sort(projectInfos)
 	t.Log(strings.Join(projectInfos, "\n"))
 	var deleteErrors int
+	var emptyProjectCount int
 	for name, projectID := range projectsToDelete {


should we adjust the log to mention the current behaviour? "deleting project resources and optionally delete project"
same for variable projectsToDelete maybe projectsToCleanAndDelete makes it more clear?

Good idea. Changed in 975741b

…roject resource management

refactor: support empty prefix for projects that shouldn't be deleted…

51e14c3

… but have resources removed

github-actions bot added the bug label Apr 10, 2025

lantoli reviewed Apr 11, 2025

View reviewed changes

internal/testutil/clean/org_clean_test.go Outdated Show resolved Hide resolved

AgustinBettati reviewed Apr 11, 2025

View reviewed changes

github-actions bot added the stale label Apr 17, 2025

github-actions bot closed this Apr 20, 2025

EspenAlbert added the not_stale Not stale issue or PR label Apr 21, 2025

EspenAlbert reopened this Apr 21, 2025

refactor: Remove resources from projects with test-acc-tf-p-keep (o…

819a31d

…nly found a few stream-instances for now)

EspenAlbert changed the title ~~refactor: Supports empty prefix for projects that shouldn't be deleted but have resources removed~~ refactor: Supports cleaning resources for static projects with test-acc-tf-p-keep prefix Apr 22, 2025

EspenAlbert marked this pull request as ready for review April 22, 2025 15:50

EspenAlbert requested a review from a team as a code owner April 22, 2025 15:50

lantoli reviewed Apr 22, 2025

View reviewed changes

internal/testutil/clean/org_clean_test.go Outdated Show resolved Hide resolved

lantoli reviewed Apr 22, 2025

View reviewed changes

internal/testutil/clean/org_clean_test.go Outdated Show resolved Hide resolved

lantoli reviewed Apr 22, 2025

View reviewed changes

EspenAlbert and others added 2 commits April 23, 2025 08:41

Update internal/testutil/clean/org_clean_test.go

6fb0e44

Co-authored-by: Leo Antoli <430982+lantoli@users.noreply.github.com>

refactor: Use a bool for skipProjectDelete

47ad1b7

AgustinBettati approved these changes Apr 23, 2025

View reviewed changes

lantoli approved these changes Apr 23, 2025

View reviewed changes

refactor: Rename projectsToDelete to projectsToClean for clarity in p…

975741b

…roject resource management

EspenAlbert merged commit c51a860 into master Apr 25, 2025
40 checks passed

EspenAlbert deleted the clean_project_resources branch April 25, 2025 05:12

		@@ -204,11 +211,6 @@ func removeProjectResources(ctx context.Context, t *testing.T, dryRun bool, clie
		}

		func projectSkipReason(p *admin.Group, skipProjectsAfter time.Time, onlyEmpty bool) string {

Conversation

EspenAlbert commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Sweeper Alternative considered

Type of change:

Required Checklist:

Further comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 17, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AgustinBettati left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EspenAlbert commented Apr 10, 2025 •

edited

Loading

AgustinBettati left a comment •

edited

Loading